Remove redundant information in `rustc_abi::Variants` by moulins · Pull Request #151742 · rust-lang/rust

moulins · 2026-01-27T14:26:28Z

Follow-up to #151040; partially addresses #113988.

Replaces the nested LayoutData in Variants::Multiple by a new, smaller VariantLayout struct, and adjust LayoutData::for_variantand the layout algorithm in consequence.
This PR is best reviewed commit-by-commit.

rustbot · 2026-01-27T14:26:32Z

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

rustbot · 2026-01-27T14:26:35Z

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

moulins · 2026-01-27T14:28:30Z

+#[derive(PartialEq, Eq, Hash, Clone, Debug)]
+#[cfg_attr(feature = "nightly", derive(HashStable_Generic))]
+pub struct VariantLayout<FieldIdx: Idx> {
+    pub size: Size,


Can't be removed, as it is used by the variant_size_differences lint.

moulins · 2026-01-27T14:31:15Z

+#[cfg_attr(feature = "nightly", derive(HashStable_Generic))]
+pub struct VariantLayout<FieldIdx: Idx> {
+    pub size: Size,
+    pub backend_repr: BackendRepr,


I don't know enough about codegen to confidently say if it requires accurate reprs for enum variants, so I've left this field for now.

Enum variants should not need their own BackendRepr, but it is probably a good idea to leave this cleanup for a future PR.

moulins · 2026-01-27T14:33:53Z

+    pub size: Size,
+    pub backend_repr: BackendRepr,
+    pub field_offsets: IndexVec<FieldIdx, Size>,
+    fields_in_memory_order: IndexVec<u32, FieldIdx>,


Note: this field could be removed and recomputed from the field offsets in Layout::from_variant; it's unclear whether this is worth it.

moulins · 2026-01-27T14:35:01Z

+                // Remove discriminant values of the other variants from the largest niche. This assumes
+                // that the largest niche, when it exists, always corresponds to the enum discriminant.


This assumption was already implicitly made by the original code.

moulins · 2026-01-27T14:38:24Z

                let min = valid_range.start.min(valid_range.end);
                let min = tag.size(cx).truncate(min);

                let max = valid_range.start.max(valid_range.end);
                let max = tag.size(cx).truncate(max);


I think this logic is broken for valid ranges wrapping around uN::MAX? In any case, fixing this is out-of-scope of the PR, so I've left it as-is.

Maybe leave a FIXME comment?

Kobzol · 2026-01-28T08:03:09Z

@bors try @rust-timer queue

Remove redundant information in `rustc_abi::Variants`

rust-bors · 2026-01-28T10:17:01Z

☀️ Try build successful (CI)
Build commit: 968e542 (968e5428d360caed1ceef0036386d8f8959a91bf, parent: e96bb7e44fbcc23c1e6009e8d0ee8ab208668fb4)

rust-timer · 2026-01-28T10:58:24Z

Finished benchmarking commit (968e542): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.3%, -0.0%]	6
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 1.4%, secondary -2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.8%	[1.6%, 5.7%]	4
Regressions ❌ (secondary)	1.9%	[0.5%, 3.4%]	3
Improvements ✅ (primary)	-1.7%	[-2.0%, -1.4%]	3
Improvements ✅ (secondary)	-4.7%	[-5.6%, -2.4%]	7
All ❌✅ (primary)	1.4%	[-2.0%, 5.7%]	7

Cycles

Results (secondary 3.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.9%	[3.9%, 5.5%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.2%	[-2.2%, -2.2%]	1
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.0%	[0.0%, 0.0%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	1

Bootstrap: 470.891s -> 473.88s (0.63%)
Artifact size: 383.44 MiB -> 383.70 MiB (0.07%)

apiraino · 2026-03-26T12:34:01Z

r? compiler

cc @makai410 since you approved #151040?

makai410 · 2026-03-26T13:36:06Z

cc @workingjubilee IIRC you were involved in the related discussion on discord?

chenyukang · 2026-03-26T13:47:37Z

@rustbot reroll

jieyouxu · 2026-03-27T10:24:08Z

I'm finding another compiler reviewer for this PR.

Remove redundant information in `rustc_abi::Variants` try-job: aarch64-msvc-1

rust-bors · 2026-04-29T03:27:08Z

☀️ Try build successful (CI)
Build commit: e4054fe (e4054fed3618c5e2897e79d885658f67ef081bb4, parent: 37d85e592f9ae5f20f7d9a9f99785246fa7298da)

saethlin · 2026-04-29T03:46:52Z

@bors r+

rust-bors · 2026-04-29T03:46:56Z

📌 Commit 93db272 has been approved by saethlin

It is now in the queue for this repository.

Remove redundant information in `rustc_abi::Variants` Follow-up to rust-lang#151040; partially addresses rust-lang#113988. Replaces the nested `LayoutData` in `Variants::Multiple` by a new, smaller `VariantLayout` struct, and adjust `LayoutData::for_variant`and the layout algorithm in consequence. This PR is best reviewed commit-by-commit.

Rollup of 9 pull requests Successful merges: - #151742 (Remove redundant information in `rustc_abi::Variants`) - #155856 (std_detect: support detecting more features on aarch64 Windows) - #155861 (Suggest `[const] Trait` bounds in more places) - #155899 (`dlltool`: Set the working directory to workaround `--temp-prefix` bug) - #155916 (Update with new LLVM 22 target for `wasm32-wali-linux-musl` target) - #155935 (remap OUT_DIR paths to fix build script path leakage in crate metadata. ) - #155950 (use the new `//@ needs-asm-mnemonic: ret` more) - #155949 (Update `opt_ast_lowering_delayed_lints` query to allow "stealing" lints, allowing to use `FnOnce` instead of `Fn`) - #155951 (Make `FlatMapInPlaceVec` an unsafe trait.)

Remove redundant information in `rustc_abi::Variants` Follow-up to #151040; partially addresses #113988. Replaces the nested `LayoutData` in `Variants::Multiple` by a new, smaller `VariantLayout` struct, and adjust `LayoutData::for_variant`and the layout algorithm in consequence. This PR is best reviewed commit-by-commit.

rust-log-analyzer · 2026-04-29T13:16:57Z

The job x86_64-gnu-llvm-21-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

test expr_store::scope::tests::while_let_adds_binding ... ok
test expr_store::tests::body::async_fn_weird_param_patterns ... ok
test expr_store::tests::body::block::inner_item_smoke ... ok
test expr_store::tests::body::block::legacy_macro_items ... ok
error: test failed, to rerun pass `-p hir-def --lib`

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/hir_def-efef625bc23d7e18 '--skip=tests::smoke_test_real_sysroot_cargo' --skip=check_code_formatting -Z unstable-options --format json` (signal: 11, SIGSEGV: invalid memory reference)
Bootstrap failed while executing `--stage 2 test --skip tests --skip coverage-map --skip coverage-run --skip library --skip tidyselftest`
Build completed unsuccessfully in 1:03:17
  local time: Wed Apr 29 13:16:43 UTC 2026
  network time: Wed, 29 Apr 2026 13:16:44 GMT
##[error]Process completed with exit code 1.

rust-bors · 2026-04-29T13:17:36Z

💔 Test for cad8c01 failed: CI. Failed job:

auto - x86_64-gnu-llvm-21-1 (web logs, enhanced plaintext logs)

JonathanBrouwer · 2026-04-29T13:31:04Z

The rollup failed with the same error, so not spurious
#155960 (comment)

JonathanBrouwer · 2026-04-29T13:41:02Z

And the try job on the rollup just succeeded, fascinating. I'll leave it to y'all to figure out what to do with this
#155960 (comment)

saethlin · 2026-04-29T13:57:44Z

SIGSEGV from unit tests wat

JonathanBrouwer · 2026-04-29T14:02:06Z

@bors try jobs=x86_64-gnu-llvm-21-1
lets get one more datapoint

Remove redundant information in `rustc_abi::Variants` try-job: x86_64-gnu-llvm-21-1

rust-bors · 2026-04-29T15:12:23Z

💔 Test for 27a14b2 failed: CI. Failed job:

try - x86_64-gnu-llvm-21-1 (web logs, enhanced plaintext logs)

rust-log-analyzer · 2026-04-29T15:12:24Z

The job x86_64-gnu-llvm-21-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

test expr_store::tests::body::array_element_cfg ... ok
test expr_store::scope::tests::test_resolve_local_name_shadow ... ok
test expr_store::scope::tests::while_let_adds_binding ... ok
test expr_store::tests::body::async_fn_weird_param_patterns ... ok
error: test failed, to rerun pass `-p hir-def --lib`

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/hir_def-efef625bc23d7e18 '--skip=tests::smoke_test_real_sysroot_cargo' --skip=check_code_formatting -Z unstable-options --format json` (signal: 11, SIGSEGV: invalid memory reference)
Bootstrap failed while executing `--stage 2 test --skip tests --skip coverage-map --skip coverage-run --skip library --skip tidyselftest`
Build completed unsuccessfully in 1:03:08
  local time: Wed Apr 29 15:12:12 UTC 2026
  network time: Wed, 29 Apr 2026 15:12:12 GMT
##[error]Process completed with exit code 1.

moulins · 2026-04-29T16:15:25Z

I can't reproduce this on my machine (linux, x86) with ./x --stage 2 test src/tools/rust-analyzer, so I don't have a clue to what's going on here :(

saethlin · 2026-05-04T00:27:29Z

The job that failed at least the last time above sets -Zrandomize-layout:

rust/src/ci/docker/host-x86_64/x86_64-gnu-llvm-21/Dockerfile

Line 54 in ad3a598

--set rust.randomize-layout=true \

In general when trying to reproduce a CI failure, it is good to run the CI scripts. Yes they can be very slow because they don't reuse your normal build artifacts, but they pull in all the configuration. You should be able to run CI locally, the docs for how to do that are here right now: https://rustc-dev-guide.rust-lang.org/tests/ci.html#docker

rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Jan 27, 2026

rustbot assigned wesleywiser Jan 27, 2026

moulins commented Jan 27, 2026

View reviewed changes